-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor offer/trade related classes in core and desktop #4672
Refactor offer/trade related classes in core and desktop #4672
Conversation
These refactoring changes are for reducing existing and potential duplication coming with the addition of new trading protocol support in the gRPC API. Some minor styling and logic simplification changes are also include. - Convert OfferUtil to injected singleton, and move various offer related utility methods into it. - Delete both MakerFeeProvider classes, which were wrappers around the same static old OfferUtil method. - Inject OfferUtil into CreateOfferDataModel, CreateOfferViewModel, TakeOfferDataModel, TakeOfferViewModel, MutableOfferDataModel, MutableOfferViewModel, OfferDataModel, EditOfferDataModel, EditOfferViewModel - Refactor TakeOfferViewModel Use OfferUtil, remove unused fields & methods. Made minor logic simplification, style and formatting changes. - MutableOfferDataModel Made minor logic simplification, style and formatting changes. - MutableOfferView uses new paymentAccount.isHalCashAccount(). - MutableOfferViewModel Refactored to use new VolumeUtil, CoinUtil, OfferUtil. Removed unused fields & accessors. Made minor style change. - Refactored OfferDataModel to use new OfferUtil - Refactor CreateOfferService Inject and use OfferUtil Move some utility methods to OfferUtil Remove unused fields - Offer Refactored to use new VolumeUtil for volume calculations. Made stateProperty and errorMessageProperty fields private. - PaymentAccount Moved isHalCashAccount type check to this class. Moved getTradeCurrency logic to this class. - Contract, radeStatistics2, TradeStatistics3 Refactored to use new VolumeUtil for volume calculations. - Trade Refactored to use new VolumeUtil for volume calculations. Made minor logic simplification, style and formatting changes. - CoinUtil Moved some coin utility methods into this class - CoinUtilTest Moved (coin related) tests from CoinCryptoUtilsTest and OfferUtilTest into CoinUtilTest, and deleted OfferUtilTest, CoinCryptoUtilsTest. - Adjust create and edit offer tests to model refactoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment. I think it's otherwise good.
To make it easier to review, it helps to keep refactorings separated in different commits. In particular pure refactor work and minor improvements are very helpful when kept separate.
desktop/src/main/java/bisq/desktop/main/offer/MutableOfferDataModel.java
Show resolved
Hide resolved
Resolves issue found during bisq-network#4672 review, and mentioned in comment bisq-network#4672 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
These refactoring changes are for reducing existing and potential duplication coming with the addition of new trading protocol support in the gRPC API. Some minor styling and logic simplification changes are also included.
Convert OfferUtil to injected singleton, and move various offer related utility methods into it.
Delete both MakerFeeProvider classes, which were wrappers around the same static old OfferUtil method.
Inject OfferUtil into CreateOfferDataModel, CreateOfferViewModel, TakeOfferDataModel, TakeOfferViewModel, MutableOfferDataModel, MutableOfferViewModel, OfferDataModel, EditOfferDataModel, EditOfferViewModel
Refactor TakeOfferViewModel
Use OfferUtil, remove unused fields & methods.
Made minor logic simplification, style and formatting changes.
MutableOfferDataModel
Made minor logic simplification, style and formatting changes.
MutableOfferView uses new paymentAccount.isHalCashAccount().
MutableOfferViewModel
Refactored to use new VolumeUtil, CoinUtil, OfferUtil.
Removed unused fields & accessors.
Made minor style change.
Refactored OfferDataModel to use new OfferUtil
Refactor CreateOfferService
Inject and use OfferUtil
Move some utility methods to OfferUtil
Remove unused fields
Offer
Refactored to use new VolumeUtil for volume calculations.
Made stateProperty and errorMessageProperty fields private.
PaymentAccount
Moved isHalCashAccount type check to this class.
Moved getTradeCurrency logic to this class.
Contract, TradeStatistics2, TradeStatistics3
Refactored to use new VolumeUtil for volume calculations.
Trade
Refactored to use new VolumeUtil for volume calculations.
Made minor logic simplification, style and formatting changes.
CoinUtil
Moved some coin utility methods into this class
CoinUtilTest
Moved (coin related) tests from CoinCryptoUtilsTest and OfferUtilTest
into CoinUtilTest, and deleted OfferUtilTest, CoinCryptoUtilsTest.
Adjust create and edit offer tests to model refactoring